Ports - Angela & Stephanie #3
Conversation
AdagramsWhat We're Looking For
Nice work on this project. Your code is readable and has good code style; well done on that! There are a few places that I think aren't quite doing what you want to be doing, so I'm adding a few comments on that. Overall, your code looks good and I see no red flags. Good work! |
| "Z", | ||
| ] | ||
|
|
||
| hand = pool[1..98].sample(10) |
There was a problem hiding this comment.
Why do you have [1..98]? What does this do? The tests still pass with this part taken out
|
|
||
| split_word.each do |letter| | ||
| index = letters_in_hand.index(letter) | ||
| if letters_in_hand.all? { letter } && index != nil |
There was a problem hiding this comment.
What does the letters_in_hand.all? { letter } part do? This code actually just checks if letter is not nil or false! Sometimes, it's good to check that letters_in_hand has no nil or false elements, but in this case, I think it's okay to assume that won't be the case. In general, just checking if index != nil (aka, if index is not nil, it's because it found something in the last line) probably suffices
|
|
||
| def is_in_english_dict?(input) | ||
| CSV.open("assets/dictionary-english.csv", "r").each do |word| | ||
| return true if word.include?(input) |
There was a problem hiding this comment.
Hm, is word.include?(input) the logic you want? What if input is shoe and word is horseshoe? Of course, in this case, shoe would probably be in the dictionary, but the line word.include?(input) is still probably not the logic you want here.
| return true if word.include?(input) | ||
| end | ||
| return false | ||
| end |
There was a problem hiding this comment.
Nicely done on the optional method! ... where are the tests to make sure it actually works? ;)
Adagrams
Congratulations! You're submitting your assignment.
Comprehension Questions
Enumerablemixin? If so, where and why was it helpful?